Skip to content

fix/scroll behavior#872

Closed
revolist wants to merge 2 commits into
mainfrom
fix/last-1mln-row
Closed

fix/scroll behavior#872
revolist wants to merge 2 commits into
mainfrom
fix/last-1mln-row

Conversation

@revolist

@revolist revolist commented May 27, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Tests

    • Added end-to-end tests for virtualization edge cases, including scrolling to the final row in large datasets (up to 1 million rows) and verifying correct positioning and visibility.
    • Added unit tests for viewport coordinate boundary calculations and scroll dimension edge cases to improve coverage.

  • Bug Fixes

    • Improved viewport coordinate handling logic for accurate render offset calculations at scroll boundaries.

Review Change Stack

@revolist

revolist commented May 27, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@coderabbitai

coderabbitai Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0b67cb79-28a7-458c-aa59-084278a02331

📥 Commits

Reviewing files that changed from the base of the PR and between 3f8f959 and c4ebf84.

📒 Files selected for processing (4)
  • e2e/virtualization.spec.ts
  • src/services/dimension.provider.ts
  • test/scroll.dimension.spec.ts
  • test/viewport.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/viewport.spec.ts

Walkthrough

This PR refactors viewport coordinate clamping from a helper function to inline logic in dimension.provider.ts, then adds unit tests for viewport max coordinate calculations and render offset behavior, and finally adds e2e tests verifying correct scrolling to the last row in both million-row and standard virtualized grids.

Changes

Viewport Coordinate Clamping Refactor

Layer / File(s) Summary
Inline viewport coordinate clamping
src/services/dimension.provider.ts		 The setViewPortCoordinate method removes the clampViewportCoordinate helper import and instead computes renderCoordinate directly by bounding the input coordinate between 0 and scrollDimension.logicalScrollSize using Math.min(Math.max(...)).
Unit tests for viewport max coordinate and render offset
test/viewport.spec.ts, test/scroll.dimension.spec.ts		 Unit tests validate the refactored behavior: test/viewport.spec.ts confirms getViewportMaxCoordinate computes max coordinate as realSize - viewportSize for overflow scenarios and remains 0 when content fits; test/scroll.dimension.spec.ts validates that renderOffset computed at the logical scroll bottom is correctly stored in both viewport and dimension stores.
E2e virtualization scrolling tests
e2e/virtualization.spec.ts		 E2e tests verify the clamping fix works end-to-end with two scenarios: million-row mode scrolls to the final row and confirms the last cell is fully contained within the viewport; normal scroll mode scrolls to the last row in a 100-row grid and validates the maximum rendered row index and bounding box alignment.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • revolist/revogrid#832: Both PRs extend e2e/virtualization.spec.ts to add Playwright virtualization scrolling assertions, building on earlier E2E test scaffolding.
  • revolist/revogrid#854: Both PRs modify e2e/virtualization.spec.ts to validate scrolling behavior to large row indices and asserting rendered cell alignment within the viewport.
  • revolist/revogrid#831: Both PRs expand e2e/virtualization.spec.ts with enhanced Playwright viewport assertion coverage for virtualized grids.

Suggested labels

bug

Suggested reviewers

  • m2a2x

Poem

🐰 A viewport clamped with care,
No helper needed anywhere,
Math.min and max align,
From bottom row to viewport line—
A million rows scroll without a tear! 🎉

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix/scroll behavior' is generic and vague; it doesn't clearly specify which scroll behavior is being fixed or what the actual changes accomplish. Consider a more descriptive title like 'Fix scroll visibility for edge cases (last row)' or 'Fix viewport coordinate clamping for extreme scroll positions' to better convey the specific scroll behavior being addressed.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/last-1mln-row

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented May 27, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@sonarqubecloud

sonarqubecloud Bot commented May 27, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@revolist revolist closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@revolist @revomanik